-
Notifications
You must be signed in to change notification settings - Fork 465
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Collapsible traces with messages #1448
Conversation
@gebner This is great. Thanks for working on this. I didn't have time to review it yet. BTW, the PR is currently marked as a draft, is something missing? |
!bench |
Here are the benchmark results for commit f722465. Benchmark Metric Change
==============================
+ stdlib size lines C -1% |
This is awesome! Could you comment on whether there is a clear path to implementing Daniel's proposal later as well? |
Is it possible to implement something like Daniel suggested (if I understand it correctly, a |
Probably not much, but the suggestion of composable filters would be extremely useful:
With the trace explorer, the ideal UI might be something like a fuzzy search input box where we can type in |
Any thoughts on what the ideal RPC API for that would be? With this PR there's only a Specifying the filters on the Lean side is much easier of course, and requires less API work. Something like |
I wanted some discussion on the RPC API first (see exchange with Wojciech). There's also plenty of trace messages to change so that they fit better into the new concept. The only module I've really touched is the TC synthesizer, the rest is just the minimum to get rid of |
It looks like this could also serve as the basis for a metaprogram that essentially tests a trace against a spec, right? The idea here is that I'd like to step through some instance searches in Functional Programming in Lean to help readers get an idea of how |
Sadly no, you probably have a better idea of it by now.
The tradeoff seems to be:
|
Thinking about the search API issue a bit more, I believe that all searching should happen server-side. This is great because:
We can implement this without breaking backwards compatibility by adding an additional field to the MsgEmbed for traces, namely |
@david-christiansen Indeed, what you describe is a bit easier now since there is a structured trace representation so you don't need to search for the regex The trace messages are also not stable, and will be improved over time. |
I've changed the serialization of |
Being unstable and improved over time seems to be a common feature of Lean 4 :-) Thanks! |
This PR contains a number of improvements to trace messages, most notably collapsible trace nodes can now have a message. Before (neovim screenshot):
After:
Rough overview of changes:
MessageData
now has a.trace
constructor. Nested traces are represented as.trace _ _ #[.trace .., .trace ..]
. And there's a new RPC calllazyTraceChildrenToInteractive
. I avoided mutually inductive types because that's painful with the derive handlers.withTraceNode `trace.class msg do ...
wheremsg : Except ε α → m MessageData
is a function that can produce different messages depending on the return value.traceCtx
andwithNestedTraces
is removed, including the feature wheretraceCtx
would drop all trace messages in its body if its trace option was not enabled.set_option trace.foo true
no longer automatically impliesset_option trace.foo.bar true
. Inheritance can be enabled---after careful consideration---on a case-by-case basis by callingregisterTraceClass `foo.bar (inherited := true)
.To try out this PR, you need a compatible editor plugin:
When editing the trace messages, I've tried to follow some key principles:
[trace.class]
marker functions as a visual bullet point, making it easy to identify the different steps at a glance.Lean.Util.Trace
help with a consistent emoji language.set_option trace.Meta.synthInstance true
(etc.) should produce great trace messages out-of-the-box, without needing extra options to tweak it. (That's the motivation for making inheritance opt-in.)Addresses #591.